Skip to content
This repository was archived by the owner on Jun 30, 2021. It is now read-only.

Do not add content-length if chunk reply. #29

Open
bluthen wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Do not add content-length if chunk reply. #29

bluthen wants to merge 2 commits into from

Conversation

@bluthen
Copy link

@bluthen bluthen commented Aug 24, 2017 •
edited
Loading

We shouldn't add Content-Length header if we are doing a chunked transfer. It is fine on most browser but some, like Safari, have issues with it.

See:
https://stackoverflow.com/questions/3304126/chunked-encoding-and-content-length-header
http://greenbytes.de/tech/webdav/rfc2616.html#rfc.section.4.4 (point 3)

@bluthen bluthen changed the title Does not add content-length if there is already a chunk transfer-enco… Do not add content-length if chunk reply. Aug 24, 2017
@NathanFrench
Copy link
Collaborator

NathanFrench commented Aug 27, 2017 •
edited
Loading

After looking over things, I decided to rework a bunch of stuff related to this.

From rfc7230 section 3.3.2 (https://tools.ietf.org/html/rfc7230#section-3.3.2), there are a lot of things we seem to be missing:

   A server MUST NOT send a Transfer-Encoding header field in any
   response with a status code of 1xx (Informational) or 204 (No
   Content).  A server MUST NOT send a Transfer-Encoding header field in
   any 2xx (Successful) response to a CONNECT request

   A server MUST NOT send a
   response containing Transfer-Encoding unless the corresponding
   request indicates HTTP/1.1 (or later).

   A server MAY send a Content-Length header field in a response to a
   HEAD request (Section 4.3.2 of [RFC7231]); a server MUST NOT send
   Content-Length in such a response unless its field-value equals the
   decimal number of octets that would have been sent in the payload
   body of a response if the same request had used the GET method.

And one that seems to relate the most to this patch

   A sender MUST NOT send a Content-Length header field in any message
   that contains a Transfer-Encoding header field.

If we look at https://github.com/criticalstack/libevhtp/blob/develop/parser.c#L1535 I'd say we could use this information, expose a header type enum, and we don't have to strcmp twice.

Thank you for pointing this out, I have a bit of work ahead of me :)

@NathanFrench NathanFrench added this to the v1.2.13 milestone Nov 2, 2017
@NathanFrench NathanFrench removed this from the v1.2.13 milestone Nov 14, 2017
@CLAassistant
Copy link

CLAassistant commented Nov 13, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bluthen @NathanFrench @CLAassistant